Add id and comment #110
Add id and comment #110
Conversation
| self._comment = "" | ||
| else: | ||
| comment_length = name_length - (id_length + 1) | ||
| comment_start = name + id_length + 1 |
There was a problem hiding this comment.
Hm, this assumes that there’s only one whitespace between the id and the comment. I’d assume this is true most of the time, but I think we need to be a bit more defensive.
There was a problem hiding this comment.
Finally, a use for strspn!
src/dnaio/_core.pyx
Outdated
| comment_start = name + id_length + 1 | ||
| self._comment = PyUnicode_New(comment_length , 127) | ||
| memcpy(PyUnicode_DATA(self._comment), comment_start, comment_length) | ||
| # Cached but nothing is internally empty string, expose externally as None |
There was a problem hiding this comment.
I don’t get the comment. Maybe just remove? Would be ok
There was a problem hiding this comment.
I will try to make it more clear in the comment. It is a rather odd design choice so it needs one, I think.
| (SequenceRecord("name ", "A", "="), None), | ||
| (SequenceRecord("name ", "A", "="), " "), | ||
| (SequenceRecord("name", "A", "="), None), | ||
| (SequenceRecord("AotC I hate sand!", "A", "="), "I hate sand!"), |
There was a problem hiding this comment.
There was a problem hiding this comment.
Thanks, I got the Star Wars reference, but didn’t know the song (and now it’ll be stuck in my head for the rest of the day ...)
There was a problem hiding this comment.
Well you could always listen to "Last jedi knight" by the same musician to get it out of your head again ;-).
|
All done! |
|
I’m happy with this now! Let’s also make a release soon. |
You had something like this in mind?
EDIT:
fixes #10